You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Make token deduplication optional for the repo indexer - #7826
This PR solves #7825 by optionally removing the unique filter from the repository indexer analyzer.
The thing is: the current behaviour directly tracks to #3452, where the unique filter was added for the sake of space saving. So, I guess there is a case where this is an acceptable trade-off. That's why I introduced an app.ini option (REPO_INDEXER_DEDUPLICATE) to make the change optional to avoid introducing problems to users that count on this.
However, I didn't notice much of a setback in the index size with this change in my tests (the resulting index was even a little smaller in my case):
With the unique filter active (no patch): 26,935,296
With the unique filter removed (with this patch): 26,136,576
I guess it really depends on the actual repository.
If the app.ini option is not desirable, I'll just remove it.
Pondering about why I didn't see the kind of improvements in size that @ethantkoenig (the author of #3452) found, one thought came to mind: could it be possible that gains he found were caused by him rebuilding the index, and not thanks to adding the unique filter? Perhaps Bleve has some kind of fragmenting issue that justifies considering rebuilding it from time to time to claim space.
Or perhaps the gains came mostly from the fact that the _all mapping was removed as well. It would be nice to test this with a larger set of repositories.
@guillep2k I think the size improvement is because removed _all mapping
@lunny I think so too, but I couldn't find anyone with a large index to try this change. If that's the case, then making the "unique" token optional makes no sense: it just should be removed. What you think?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
lgtm/need 2This PR needs two approvals by maintainers to be considered for merging.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GiteaBot
added
the
lgtm/need 2
This PR solves #7825 by optionally removing the
uniquefilter from the repository indexer analyzer.The thing is: the current behaviour directly tracks to #3452, where the
uniquefilter was added for the sake of space saving. So, I guess there is a case where this is an acceptable trade-off. That's why I introduced an app.ini option (REPO_INDEXER_DEDUPLICATE) to make the change optional to avoid introducing problems to users that count on this.However, I didn't notice much of a setback in the index size with this change in my tests (the resulting index was even a little smaller in my case):
uniquefilter active (no patch): 26,935,296uniquefilter removed (with this patch): 26,136,576I guess it really depends on the actual repository.
If the app.ini option is not desirable, I'll just remove it.